Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix parallel builds w/ LTO on systems where make is not GNU make. #13955

Merged
merged 1 commit into from
Nov 5, 2021
Merged

Fix parallel builds w/ LTO on systems where make is not GNU make. #13955

merged 1 commit into from
Nov 5, 2021

Conversation

uqs
Copy link

@uqs uqs commented Aug 10, 2021 •
edited
Loading

When compiling with LTO, the lto-wrapper of GCC will detect the presence
of parallelism via the environment, and either call $MAKE or simply
make (and the detection of a jobserver messes this up for FreeBSD).

On FreeBSD and other systems, you have to build with gmake and the
way to have lto-wrapper use the same make(1) is to pass it via the
environment.

This fixes the (parallel) linking when LTO is active under FreeBSD.

Description

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@uqs
Fix parallel builds w/ LTO on systems where make is not GNU make.
a57d336 
When compiling with LTO, the lto-wrapper of GCC will detect the presence
of parallelism via the environment, and either call $MAKE or simply
`make` (and the detection of a jobserver messes this up for FreeBSD).

On FreeBSD and other systems, you have to build with `gmake` and the
way to have lto-wrapper use the same make(1) is to pass it via the
environment.

This fixes the (parallel) linking when LTO is active under FreeBSD.
@github-actions github-actions bot added the core label Aug 10, 2021
@drashna
Copy link
Member

drashna commented Aug 11, 2021

I'm not sure how necessary this is, since qmk compile and qmk flash explicitly will call gmake if it finds it.

qmk_firmware/lib/python/qmk/commands.py

Lines 19 to 27 in 8d4fcd6

def _find_make():
"""Returns the correct make command for this environment.
"""
make_cmd = os.environ.get('MAKE')
if not make_cmd:
make_cmd = 'gmake' if shutil.which('gmake') else 'make'
return make_cmd

@uqs
Copy link
Author

uqs commented Aug 11, 2021

That is not the issue here. In fact, I don't use the qmk wrapper, but will invoke gmake directly.

The problem is that gmake will fork lto-wrapper, which detects a parallel make, and then it produces a temporary Makefile and then it wants to fork $MAKE (falling back to make if not present). And it uses incompatible args to do so, so then BSD make barfs and the linking fails. See https://github.com/gcc-mirror/gcc/blob/master/gcc/lto-wrapper.c#L1379-L1383

So it's the compiler/linker that is forking make, not the other way round. ¯_(ツ)_/¯

A workaround for me is to run it like so: gmake -j4 --output-sync handwired/dactyl_manuform/5x6:uqs MAKE=gmake
but that is a bit silly. We could also have the qmk wrapper put MAKE=gmake in the environment, but then users of plain gmake would see random breakage.

One way or the other, we need to clue in lto-wrapper to use GNU make, or disable its parallelism outright, so it stops forking make.

@stale
Copy link

stale bot commented Sep 26, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@uqs
Copy link
Author

uqs commented Sep 27, 2021

This is still a problem that would be fixed by merging the request. How can I stop the stalebot?

@stale stale bot removed the awaiting changes label Sep 27, 2021
@tzarc tzarc changed the base branch from master to develop November 1, 2021 22:05
@tzarc tzarc requested a review from a team November 1, 2021 22:06
@0mp
Copy link

0mp commented Nov 5, 2021

That's a cool change, thank you!

@drashna drashna merged commit 5bb5bb1 into qmk:develop Nov 5, 2021
ptrxyz pushed a commit to ptrxyz/qmk_firmware that referenced this pull request Apr 9, 2022
@uqs @ptrxyz
Fix parallel builds w/ LTO on systems where make is not GNU make. (qm…
696ccaf 
…k#13955)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tzarc tzarc tzarc approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
bug core in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@uqs @drashna @0mp @tzarc